Skip to content

fix(agent): harden sub-agent streamline (PR #582 follow-up)#584

Merged
OisinKyne merged 1 commit into
oisin/sell-agent-streamlinefrom
fix/sub-agent-streamline-hardening
Jun 3, 2026
Merged

fix(agent): harden sub-agent streamline (PR #582 follow-up)#584
OisinKyne merged 1 commit into
oisin/sell-agent-streamlinefrom
fix/sub-agent-streamline-hardening

Conversation

@bussyjd
Copy link
Copy Markdown
Contributor

@bussyjd bussyjd commented Jun 3, 2026

Summary

Follow-up hardening for #582 ("Streamline sub-agents"), based on oisin/sell-agent-streamline so it can merge into that branch before #582 lands. Addresses three review findings; no behavior from #582 is reverted — this tightens the contract and adds the coverage that was missing.

Changes

#2 — make the "sub-agents only" marker invariant enforced, not just convention.
The .no-bundled-skills marker is written unconditionally inside SeedHostFiles. It's safe today because all callers are sub-agent paths and the master seeds via internal/hermes, but nothing guarded it. Added a doc-contract on SeedHostFiles plus TestMarkerOnlyWrittenBySeedHostFiles, which proves the marker is written only by SeedHostFiles — never by the reusable seed primitives (WriteSoul, embed.WriteSkillSubset) a master/objective-only path could route through. A future refactor that pushes the write into a shared primitive fails this test first.

#3 — fix the terminal.timeout / lifetime_seconds inversion.
The rendered sub-agent config had timeout: 180 (per-operation) larger than lifetime_seconds: 90 (whole session) — a single op was nominally allowed to outlive the session. Lowered terminal.timeout to 80 (≤ 90, well under the 100s Cloudflare free-tunnel cap). The render test now parses both values and asserts timeout <= lifetime_seconds rather than only checking literals, so the relation is enforced even if the numbers drift.

#1 — verify the Hermes image actually honors the streamlined contract.
Existing unit tests only assert our rendered output contains the marker + capped keys; nothing proved nousresearch/hermes-agent:v2026.5.28 respects them. Added a //go:build integration test that deploys a real CRD sub-agent via obol agent new and asserts end-to-end:

  1. the .no-bundled-skills marker on the host PVC path and visible inside the pod;
  2. the capped hermes-config keys (lifetime_seconds: 90, max_turns: 30, reasoning_effort: low, disabled_toolsets: [memory, web]);
  3. a behavioral bundled-skills-skipped signal — obol-skills external dir populated while the native bundled-skills dir is absent/empty (chosen over a brittle log-grep on an external image's wording).

It t.Skips gracefully without a cluster, consistent with the rest of the integration suite. The optional in-pod inference exercise is gated on OBOL_LLM_ENDPOINT.

Docs. plans/sell-agent-perf.md gets a post-review hardening section and corrected measured figures (SOUL.md template 2128 → 1460 bytes / ~370 tok — the earlier "~1050 → ~500 tok" was optimistic; addresses split preserved all 177 unique addresses across 8 reference files, verified by diff).

Test evidence

gofmt -l (touched pkgs) → clean
go build ./...           → OK
go test ./internal/agentcrd/ ./internal/serviceoffercontroller/ ./internal/hermes/ → ok
  TestMarkerOnlyWrittenBySeedHostFiles       → PASS (3 subtests)
  TestRenderHermesConfig_SubAgentConstraints → PASS
go vet -tags integration ./internal/agentcrd/ ./internal/serviceoffercontroller/ → OK (integration test compiles)

Notes

  • The master agent's config still renders timeout: 180 / lifetime: 300 — intentionally untouched: it isn't tunnel-exposed and 180 ≤ 300 is internally coherent, so User facing ingress #3's tunnel rationale doesn't apply.
  • To run the integration test live: go test -tags integration -run TestIntegration_AgentContract -v -timeout 15m ./internal/agentcrd/ against a cluster from obol stack up.

Addresses review findings #1/#2/#3 on PR #582 "Streamline sub-agents".

- agentcrd: document the SeedHostFiles "sub-agents-for-sale only" contract
  and add TestMarkerOnlyWrittenBySeedHostFiles, locking that the
  .no-bundled-skills marker is written only by SeedHostFiles and never by
  the reusable seed primitives (WriteSoul, embed.WriteSkillSubset) a master
  or objective-only path could route through. [finding #2]
- serviceoffercontroller: lower the rendered terminal.timeout 180 -> 80 so a
  single operation can no longer outlive the 90s lifetime_seconds session;
  the render test now parses both and asserts timeout <= lifetime_seconds. [finding #3]
- agentcrd: add a //go:build integration test that deploys a real CRD
  sub-agent and verifies the Hermes image honors the contract end-to-end:
  the marker on the PVC and visible in-pod, the capped hermes-config keys
  (lifetime_seconds/max_turns/reasoning_effort/disabled_toolsets), and a
  behavioral bundled-skills-skipped signal. Skips gracefully without a
  cluster. [finding #1]
- plans/sell-agent-perf.md: post-review hardening section and corrected
  measured figures (SOUL.md 2128 -> 1460 bytes ~370 tok; 177 addresses
  preserved across 8 reference files).
@bussyjd bussyjd requested a review from OisinKyne June 3, 2026 13:26
@OisinKyne OisinKyne merged commit fe47093 into oisin/sell-agent-streamline Jun 3, 2026
@OisinKyne OisinKyne deleted the fix/sub-agent-streamline-hardening branch June 3, 2026 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants